Skip to content

Engineering: coil-aware port placement constraint#298

Open
krystophny wants to merge 2 commits intomainfrom
feature/port-coil-clearance
Open

Engineering: coil-aware port placement constraint#298
krystophny wants to merge 2 commits intomainfrom
feature/port-coil-clearance

Conversation

@krystophny
Copy link
Copy Markdown
Member

@krystophny krystophny commented Jan 5, 2026

User description

Adds a coil-clearance constraint to the Python PortOptimizer (maps candidate port locations to wall xyz via chartmap and rejects ports too close to coil segments). Also adds an engineering test plot artifact and parallelizes the chartmap wall-losses integration test over particles.

Validation:

  • make test TEST=test_engineering (passed)
  • make test TEST=test_chartmap_wall_losses (passed)
  • make test TEST=test_chartmap_ncsx_map2disc_wall_offset (passed)

Test artifacts (local build output paths):

  • build/test/python/test_artifacts/engineering/port_feasibility_coil_clearance.png
  • build/test/tests/chartmap_wall_losses_summary.png
  • build/test/tests/chartmap_wall_offset_check.png

PR Type

Enhancement, Tests


Description

  • Add coil-clearance constraint to PortOptimizer for geometric feasibility

    • Maps candidate port locations to wall xyz via chartmap
    • Rejects ports violating minimum distance to coil segments
  • Parallelize chartmap wall-losses integration test with OpenMP

    • Add OpenMP directives to all particle tracing subroutines
    • Report max thread count in test output
  • New coil_clearance module with wall surface interpolation and distance queries

  • Add visualization function for heat flux with coil clearance overlay


Diagram Walkthrough

flowchart LR
  A["Port Optimizer"] -->|set_coil_clearance_constraint| B["CoilClearanceConstraint"]
  B -->|loads| C["CoilSegments from file"]
  B -->|loads| D["WallSurface from chartmap"]
  B -->|checks| E["port_violates"]
  E -->|computes| F["min_distance_points_to_segments"]
  A -->|objective function| G["penalty if violation"]
  H["test_chartmap_wall_losses"] -->|OpenMP| I["parallel particle tracing"]
  I -->|speedup| J["faster integration test"]
Loading

File Walkthrough

Relevant files
Enhancement
coil_clearance.py
New coil clearance constraint module                                         

python/pysimple/coil_clearance.py

  • New module for coil-aware geometric feasibility constraints
  • Load coil points from SIMPLE/libneo-style files and split into
    polylines
  • Interpolate wall surface from chartmap NetCDF on rho=1 boundary
  • Compute minimum distances from wall points to coil segments with
    blocking
  • CoilClearanceConstraint class to check port violations
  • clearance_map_on_heatmap_grid function for visualization
+392/-0 
engineering.py
Integrate coil clearance into port optimizer                         

python/pysimple/engineering.py

  • Import CoilClearanceConstraint, CoilSegments, WallSurface from
    coil_clearance
  • Add _coil_clearance field to PortOptimizer
  • New set_coil_clearance_constraint method to configure constraint
  • Integrate coil clearance check into _objective function with 1e6
    penalty
  • New plot_heat_flux_with_coil_clearance function for visualization
+105/-0 
test_chartmap_wall_losses.f90
Parallelize particle tracing with OpenMP                                 

test/tests/magfie/test_chartmap_wall_losses.f90

  • Add omp_lib module import for OpenMP thread reporting
  • Print max OpenMP threads at test start
  • Add !$omp parallel do directives to all 8 particle tracing subroutines
  • Parallelize loops over n_particles with static scheduling
  • Properly declare private variables for each parallel region
+18/-0   
Tests
test_engineering.py
Add coil clearance tests and mock coil file generator       

test/python/test_engineering.py

  • Import plot_heat_flux_with_coil_clearance function
  • Add create_mock_coil_file helper to generate test coil polyline files
  • New TestCoilClearance test class with two test methods
  • test_coil_clearance_penalizes_forbidden_ports validates constraint
    penalty
  • test_plot_heat_flux_with_coil_clearance_writes_artifact tests
    visualization
+98/-0   
Configuration changes
CMakeLists.txt
Configure test artifacts directory for engineering tests 

test/python/CMakeLists.txt

  • Add SIMPLE_TEST_ARTIFACTS_DIR environment variable to test_engineering
  • Enables test to write artifact plots to proper output directory
+1/-1     

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Jan 5, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing input validation: WallSurface.from_chartmap assumes required NetCDF variables exist and have expected
shapes/units (e.g., x/y/z indexing [:, :, -1]) without explicit validation, so malformed
or unexpected chartmap inputs will fail with low-context exceptions (e.g., KeyError/index
errors).

Referred Code
@classmethod
def from_chartmap(cls, chartmap_file: str | Path) -> "WallSurface":
    if not HAS_NETCDF4:
        raise ImportError("netCDF4 required: pip install netCDF4")

    path = Path(chartmap_file)
    with nc.Dataset(path, "r") as ds:
        theta = np.asarray(ds.variables["theta"][:], dtype=float)
        zeta = np.asarray(ds.variables["zeta"][:], dtype=float)
        nfp = int(ds.variables["num_field_periods"][...])

        x_wall = np.asarray(ds.variables["x"][:, :, -1], dtype=float) * 0.01
        y_wall = np.asarray(ds.variables["y"][:, :, -1], dtype=float) * 0.01
        z_wall = np.asarray(ds.variables["z"][:, :, -1], dtype=float) * 0.01

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
External file handling: The new set_coil_clearance_constraint and plotting helper accept external
coil_file/chartmap_file paths and load their contents, but the diff does not show broader
system-level safeguards (trusted path constraints/sandboxing) needed to confirm
security-first handling of externally supplied files.

Referred Code
def set_coil_clearance_constraint(
    self,
    coil_file: str | Path,
    *,
    chartmap_file: str | Path,
    min_clearance_m: float,
    use_zero_current_separators: bool = True,
    jump_factor: float = 25.0,
) -> "PortOptimizer":
    """
    Enforce a minimum distance from ports to coils (geometric feasibility).

    This constraint requires a chartmap file to map (theta, zeta) port
    locations to physical-space coordinates on the wall.
    """
    if min_clearance_m <= 0.0:
        raise ValueError("min_clearance_m must be positive")

    wall = WallSurface.from_chartmap(chartmap_file)
    coils = CoilSegments.from_file(
        coil_file,


 ... (clipped 9 lines)

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Jan 5, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix periodic interpolation weight calculation

Fix the periodic interpolation weight calculation in _periodic_bilinear_indices
to correctly handle wrap-around cases by applying a modulo operation to the
numerator.

python/pysimple/coil_clearance.py [234-255]

 def _periodic_bilinear_indices(
     grid: np.ndarray, values: np.ndarray, period: float
 ) -> tuple[np.ndarray, np.ndarray, np.ndarray]:
     n = grid.size
     if n < 2:
         raise ValueError("Interpolation grid must have at least 2 points")
 
     v = np.mod(values, period)
     i1 = np.searchsorted(grid, v, side="right")
     i0 = i1 - 1
     i0 = np.mod(i0, n)
     i1 = np.mod(i1, n)
 
     g0 = grid[i0]
     g1 = grid[i1]
 
     wrapped = i1 <= i0
     denom = np.where(wrapped, (g1 + period) - g0, g1 - g0)
+    numer = np.where(wrapped, np.mod(v - g0, period), v - g0)
     denom = np.where(denom == 0.0, 1.0, denom)
-    w = (v - g0) / denom
+    w = numer / denom
     w = np.clip(w, 0.0, 1.0)
     return i0, i1, w
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a subtle bug in the periodic interpolation logic that occurs at boundary wrap-around and provides a correct fix, preventing potential errors in coordinate mapping.

Medium
Improve header parsing robustness

In load_coil_points, handle a potential IndexError during header parsing by
adding it to the except block to prevent crashes on malformed files.

python/pysimple/coil_clearance.py [27-59]

 def load_coil_points(coil_file: str | Path) -> tuple[np.ndarray, np.ndarray]:
     """
     Load coil points from a SIMPLE/libneo-style coil file.
 
     Format:
         First line: integer N (number of points)
         Next N lines: x y z I
 
     Coordinates are expected to be in meters.
 
     Returns:
         points: (N, 3) array of xyz points [m]
         currents: (N,) array of currents
     """
     path = Path(coil_file)
     with path.open("r", encoding="utf-8") as f:
         header = f.readline().strip()
         if not header:
             raise ValueError(f"Empty coil file: {path}")
         try:
             n_points = int(header.split()[0])
-        except ValueError as exc:
-            raise ValueError(f"Invalid coil file header (expected N): {path}") from exc
+        except (ValueError, IndexError) as exc:
+            raise ValueError(f"Invalid coil file header (expected N points): {path}") from exc
 
         data = np.loadtxt(f, dtype=float, ndmin=2)
     if data.shape[0] != n_points or data.shape[1] < 4:
         raise ValueError(
             f"Coil file {path} expected {n_points} rows of x y z I, got {data.shape}"
         )
 
     points = data[:, 0:3].astype(float, copy=False)
     currents = data[:, 3].astype(float, copy=False)
     return points, currents
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies a potential IndexError for malformed coil file headers and proposes a simple fix, improving the function's robustness against edge-case inputs.

Low
High-level
Use a dedicated geometry library

Instead of implementing complex geometric calculations from scratch in
coil_clearance.py, consider using a mature geometry library like trimesh or
scipy.spatial. This would simplify the code and improve its robustness and
maintainability.

Examples:

python/pysimple/coil_clearance.py [178-231]
def min_distance_points_to_segments(
    points: np.ndarray,
    p0: np.ndarray,
    p1: np.ndarray,
    *,
    block_size: int = 128,
) -> np.ndarray:
    """
    Compute minimum distances from many points to many segments.


 ... (clipped 44 lines)

Solution Walkthrough:

Before:

# file: python/pysimple/coil_clearance.py

def min_distance_points_to_segments(points, p0, p1, ...):
    """
    Compute minimum distances from many points to many segments.
    ...
    """
    # ... complex numpy implementation with einsum and broadcasting ...
    w = points[:, None, :] - p0b[None, :, :]
    t = np.einsum("mbi,bi->mb", w, v) / vv[None, :]
    t = np.clip(t, 0.0, 1.0)
    proj = p0b[None, :, :] + t[:, :, None] * v[None, :, :]
    d2 = np.einsum("mbi,mbi->mb", points[:, None, :] - proj, points[:, None, :] - proj)
    min_d2 = np.minimum(min_d2, np.min(d2, axis=1))
    return np.sqrt(min_d2)

class CoilClearanceConstraint:
    def port_violates(...):
        # ...
        xyz = self.wall.xyz(theta_pts, zeta_pts)
        d = min_distance_points_to_segments(xyz, self.coils.p0, self.coils.p1)
        return bool(np.any(d < self.min_clearance_m))

After:

# file: python/pysimple/coil_clearance.py
# Requires adding 'trimesh' as a dependency

import trimesh

def min_distance_points_to_segments(points, p0, p1, ...):
    """
    Compute minimum distances from many points to many segments using trimesh.
    """
    segments = np.stack((p0, p1), axis=1)
    path = trimesh.path.Path3D(entities=None, vertices=None, segments=segments)
    # The trimesh query returns closest points, distances, and segment indices
    _, distances, _ = path.nearest.on_path(points)
    return distances

class CoilClearanceConstraint:
    def port_violates(...):
        # ...
        xyz = self.wall.xyz(theta_pts, zeta_pts)
        d = min_distance_points_to_segments(xyz, self.coils.p0, self.coils.p1)
        return bool(np.any(d < self.min_clearance_m))
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the PR implements complex geometric calculations from scratch in coil_clearance.py, and proposes a valid architectural alternative of using a specialized library to improve robustness and maintainability.

Medium
General
Ensure consistent coil splitting parameters

To ensure consistency between optimization and visualization, update
plot_heat_flux_with_coil_clearance to accept use_zero_current_separators and
jump_factor and pass them to CoilSegments.from_file.

python/pysimple/engineering.py [1057-1117]

 def plot_heat_flux_with_coil_clearance(
     heat_map: WallHeatMap,
     *,
     coil_file: str | Path,
     chartmap_file: str | Path,
     min_clearance_m: float,
     ax=None,
     cmap: str = "hot",
     vmax: Optional[float] = None,
     show_colorbar: bool = True,
     ports: Optional[list[Port]] = None,
     forbidden_alpha: float = 0.35,
+    use_zero_current_separators: bool = True,
+    jump_factor: float = 25.0,
 ):
     """
 ... (clipped 10 lines)
     ax = plot_heat_flux_2d(
         heat_map,
         ax=ax,
         cmap=cmap,
         vmax=vmax,
         show_colorbar=show_colorbar,
         ports=ports,
     )
 
     wall = WallSurface.from_chartmap(chartmap_file)
-    coils = CoilSegments.from_file(coil_file)
+    coils = CoilSegments.from_file(
+        coil_file,
+        use_zero_current_separators=use_zero_current_separators,
+        jump_factor=jump_factor,
+    )
     clearance = clearance_map_on_heatmap_grid(
         wall,
         coils,
         heat_map.theta_centers,
         heat_map.zeta_centers,
     )
     forbidden = clearance < min_clearance_m
 
     forbidden_grid = forbidden.astype(float)
     ax.pcolormesh(
         np.degrees(heat_map.zeta_edges),
         np.degrees(heat_map.theta_edges),
-        forbidden_grid,
+        forbidden_grid.T,
         cmap="Greys",
         vmin=0.0,
         vmax=1.0,
         shading="flat",
         alpha=forbidden_alpha,
     )
     ax.set_title(
         f"Wall Heat Flux + Coil Clearance (d > {min_clearance_m:.3f} m)"
     )
     return ax

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out a potential inconsistency between optimization and visualization by allowing different coil splitting parameters, and the proposed change improves the API to ensure consistency.

Low
Ensure zip compatibility

Remove strict=True from the zip call in split_coil_polylines to ensure
compatibility with Python versions earlier than 3.10.

python/pysimple/coil_clearance.py [88]

-for p, I in zip(points, currents, strict=True):
+for p, I in zip(points, currents):
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies a Python version compatibility issue with zip(strict=True) and proposes a valid fix, though the impact is minor as the surrounding code already ensures the zipped iterables have the same length.

Low
  • Update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant